-
Notifications
You must be signed in to change notification settings - Fork 3
Add automatic fallback to coarser datetime units during inference mode #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: api-to_datetime-units
Are you sure you want to change the base?
Add automatic fallback to coarser datetime units during inference mode #4
Conversation
- Added helper function get_next_coarser_unit for unit fallback sequence - Modified array_strptime to wrap implementation with fallback logic - Modified array_to_datetime similarly - Added tests for fallback behavior - Need to fix issue with all-NaT case and ensure proper resolution handling Co-authored-by: jbrockmendel <8078968+jbrockmendel@users.noreply.github.com>
- Fixed fallback logic to retry with ns first before falling back to coarser units - Fixed unit_for_numerics handling in recursive calls - Added tests for explicit ns resolution fallback - All strptime and array_to_datetime tests now pass Co-authored-by: jbrockmendel <8078968+jbrockmendel@users.noreply.github.com>
pandas/_libs/tslib.pyx
Outdated
|
|
||
| while True: | ||
| try: | ||
| # When falling back to coarser units, don't pass unit_for_numerics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is inaccurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this is inaccurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 0f8a6ef. Updated the comment to accurately explain that unit_for_numerics is set to None during fallback so it will be computed from fallback_creso, rather than incorrectly stating it's "meant for ns resolution only".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot no. unit_for_numerics should't be set to None during fallback. just keep using whatever the input unit_for_numerics is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 8c99476. Now unit_for_numerics is always passed through unchanged - no longer set to None during fallback. The fallback logic now only applies during inference mode and uses recursion as requested.
Updated comment to correctly explain that unit_for_numerics is set to None during fallback so it will be computed from the fallback_creso, rather than incorrectly stating it's "meant for ns resolution only". Co-authored-by: jbrockmendel <8078968+jbrockmendel@users.noreply.github.com>
|
copilot suggested I write the following comment here: The fallback logic should ONLY apply during inference mode (infer_reso=True). When explicit unit is passed: NO fallback - raise OutOfBoundsDatetime as currently done. Remove wrapper functions and use recursion to match existing code style. |
Per reviewer feedback: - Fallback now ONLY applies during inference mode (creso=GENERIC) - When explicit unit is passed, OutOfBoundsDatetime is raised as before - Removed wrapper functions and use recursion to match existing code style - Keep unit_for_numerics unchanged during fallback - Added get_next_coarser_unit() helper function in both files Co-authored-by: jbrockmendel <8078968+jbrockmendel@users.noreply.github.com>
Implemented in commit 8c99476. Changes made:
|
Implementing automatic fallback to coarser units when OutOfBoundsDatetime occurs in array_strptime and array_to_datetime during inference mode only.
Summary
Successfully implemented automatic fallback to coarser datetime units when OutOfBoundsDatetime errors occur during inference mode, allowing pandas to gracefully handle dates outside the nanosecond range without changing behavior for explicit unit requests.
Changes Made
Core Implementation
get_next_coarser_unit()helper function in both strptime.pyx and tslib.pyxarray_strptimeto catch OutOfBoundsDatetime during inference and recursively retry with coarser unitsarray_to_datetimeto catch OverflowError during inference and recursively retry with coarser unitsKey Behavioral Changes
Testing
Examples
Files Modified
pandas/_libs/tslibs/strptime.pyx- Added helper and fallback logic during inferencepandas/_libs/tslib.pyx- Added helper and fallback logic during inferenceOriginal prompt
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.